Merged
Conversation
WalkthroughReplaces timestamp-based dirty checks with direct Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@tx_service/src/cc/cc_shard.cpp`:
- Around line 406-421: The assert guarding the underflow case in the dirty count
path (assert(dirty_delta >= 0 || dirty_data_key_count_ >=
static_cast<size_t>(-dirty_delta))) must be relaxed because underflow can
legitimately occur when an entry is flushed but was never counted dirty (see
OnCommittedUpdate flows); replace the assert with a runtime check that logs a
warning via the appropriate logger (mentioning dirty_data_key_count_ and
dirty_delta) instead of aborting in debug builds, then continue with the
existing clamp logic that computes new_dirty and sets dirty_data_key_count_
accordingly.
Comment on lines
+406
to
+421
| // Sanity check in debug mode. | ||
| assert(dirty_delta >= 0 || | ||
| dirty_data_key_count_ >= static_cast<size_t>(-dirty_delta)); | ||
| dirty_data_key_count_ = static_cast<size_t>( | ||
| static_cast<int64_t>(dirty_data_key_count_) + dirty_delta); | ||
| // Clamp to avoid underflow when an entry is flushed but was never | ||
| // counted dirty (e.g. became dirty via a path that doesn't call | ||
| // OnCommittedUpdate). | ||
| int64_t new_dirty = | ||
| static_cast<int64_t>(dirty_data_key_count_) + dirty_delta; | ||
| if (new_dirty < 0) | ||
| { | ||
| dirty_data_key_count_ = 0; | ||
| } | ||
| else | ||
| { | ||
| dirty_data_key_count_ = static_cast<size_t>(new_dirty); | ||
| } |
There was a problem hiding this comment.
Avoid debug-only aborts if underflow is now a tolerated path.
You clamp dirty_data_key_count_ to zero, but the existing assert will still fire in debug/test runs when the “flushed but never counted dirty” scenario happens. If this is now expected, consider downgrading the assert to a warning so debug builds don’t fail.
🔧 Proposed adjustment
- // Sanity check in debug mode.
- assert(dirty_delta >= 0 ||
- dirty_data_key_count_ >= static_cast<size_t>(-dirty_delta));
+ if (dirty_delta < 0 &&
+ dirty_data_key_count_ < static_cast<size_t>(-dirty_delta))
+ {
+ DLOG(WARNING) << "dirty_data_key_count_ underflow; clamping to 0";
+ }
// Clamp to avoid underflow when an entry is flushed but was never
// counted dirty (e.g. became dirty via a path that doesn't call
// OnCommittedUpdate).
int64_t new_dirty =
static_cast<int64_t>(dirty_data_key_count_) + dirty_delta;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Sanity check in debug mode. | |
| assert(dirty_delta >= 0 || | |
| dirty_data_key_count_ >= static_cast<size_t>(-dirty_delta)); | |
| dirty_data_key_count_ = static_cast<size_t>( | |
| static_cast<int64_t>(dirty_data_key_count_) + dirty_delta); | |
| // Clamp to avoid underflow when an entry is flushed but was never | |
| // counted dirty (e.g. became dirty via a path that doesn't call | |
| // OnCommittedUpdate). | |
| int64_t new_dirty = | |
| static_cast<int64_t>(dirty_data_key_count_) + dirty_delta; | |
| if (new_dirty < 0) | |
| { | |
| dirty_data_key_count_ = 0; | |
| } | |
| else | |
| { | |
| dirty_data_key_count_ = static_cast<size_t>(new_dirty); | |
| } | |
| // Clamp to avoid underflow when an entry is flushed but was never | |
| // counted dirty (e.g. became dirty via a path that doesn't call | |
| // OnCommittedUpdate). | |
| if (dirty_delta < 0 && | |
| dirty_data_key_count_ < static_cast<size_t>(-dirty_delta)) | |
| { | |
| DLOG(WARNING) << "dirty_data_key_count_ underflow; clamping to 0"; | |
| } | |
| int64_t new_dirty = | |
| static_cast<int64_t>(dirty_data_key_count_) + dirty_delta; | |
| if (new_dirty < 0) | |
| { | |
| dirty_data_key_count_ = 0; | |
| } | |
| else | |
| { | |
| dirty_data_key_count_ = static_cast<size_t>(new_dirty); | |
| } |
🤖 Prompt for AI Agents
In `@tx_service/src/cc/cc_shard.cpp` around lines 406 - 421, The assert guarding
the underflow case in the dirty count path (assert(dirty_delta >= 0 ||
dirty_data_key_count_ >= static_cast<size_t>(-dirty_delta))) must be relaxed
because underflow can legitimately occur when an entry is flushed but was never
counted dirty (see OnCommittedUpdate flows); replace the assert with a runtime
check that logs a warning via the appropriate logger (mentioning
dirty_data_key_count_ and dirty_delta) instead of aborting in debug builds, then
continue with the existing clamp logic that computes new_dirty and sets
dirty_data_key_count_ accordingly.
thweetkomputer
approved these changes
Feb 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Here are some reminders before you submit the pull request
fixes eloqdb/tx_service#issue_id./mtr --suite=mono_main,mono_multi,mono_basicSummary by CodeRabbit